Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable dxvknvapi on a bunch of (DLSS) titles I've verified as functioning and stable. #6120

Closed
wants to merge 3 commits into from

Conversation

adamnv
Copy link
Contributor

@adamnv adamnv commented Aug 26, 2022

Note: in some cases I've included the app ID for a game demo but not the full game. This is only because in those cases I've not tested the full game, and wanted to strictly only include titles I've directly verified as stable'n'good.

However, it's probably very fair to say that if a game demo had good DLSS/nvapi stability then the full game will also have good DLSS/nvapi stability, so if you'd like me to add the app IDs for full titles sight-unseen then I'm happy to do so! (edit: done that.)

@ivyl
Copy link
Collaborator

ivyl commented Sep 7, 2022

It would be good to have also the full versions of the game tested, but that's something we can add based on the demos.

Just to understand the extent of testing you have done, have you tried any of the games with older, non-RTX GPUs?

It should not negatively affect games running with other vendors' GPUs but it would be nice to make sure that's the case.

The change would go into experimental which already requires VK_KHR_dynamic_rendering, so that limits the scope of the backwards compatibility we would need to check for :-)

@adamnv
Copy link
Contributor Author

adamnv commented Sep 8, 2022

Thanks for the reponse! Comments inline:

It would be good to have also the full versions of the game tested, but that's something we can add based on the demos.

I hope to (re)test just about everything in the fullness of time (and steam sales 😆) but I'm happy to add the non-demos on the basis of the success with the demos; are you saying I should go ahead with that in this PR?

Just to understand the extent of testing you have done, have you tried any of the games with older, non-RTX GPUs?

On the one hand - no, I haven't! On the other hand, I'm familiar enough with the failure modes of the only dxvknvapi-enabled tech which overlaps with RTX cards - DLSS - to be adequately confident that DLSS itself fails gracefully.

It should not negatively affect games running with other vendors' GPUs but it would be nice to make sure that's the case.

I agree; FWIW I believe dxvknvapi is designed by default to do nothing outside of the NVIDIA driver stack unless a user is explicitly spoofing a customVendorId, but I'm checking that assumption again with @jp7677 😁 edit: confirmed by Jens - dxvknvapi does nothing outside of the NVIDIA driver stack by default, regardless of PROTON_ENABLE_NVAPI.

The change would go into experimental which already requires VK_KHR_dynamic_rendering, so that limits the scope of the backwards compatibility we would need to check for :-)

Would you like me to retarget the PR against another branch or is this one okay?

@jp7677
Copy link
Contributor

jp7677 commented Sep 9, 2022

I agree; FWIW I believe dxvknvapi is designed by default to do nothing outside of the NVIDIA driver stack unless a user is explicitly spoofing a customVendorId, but I'm checking that assumption again with @jp7677 grin edit: confirmed by Jens - dxvknvapi does nothing outside of the NVIDIA driver stack by default, regardless of PROTON_ENABLE_NVAPI.

Yes correct. When no found device runs on the NVIDIA proprietary driver, NvAPI_Initialize fails with NVAPI_NVIDIA_DEVICE_NOT_FOUND. This should match behavior on Windows.

(You can override the default behavior with DXVK_NVAPI_ALLOW_OTHER_DRIVERS=1 for e.g. using Reflex or NVAPI D3D11 extensions with AMD cards, see https://github.com/jp7677/dxvk-nvapi#tweaks-debugging-and-troubleshooting. Setting PROTON_ENABLE_NVAPI does not touch this switch.)

On the one hand - no, I haven't! On the other hand, I'm familiar enough with the failure modes of the only dxvknvapi-enabled tech which overlaps with RTX cards - DLSS - to be adequately confident that DLSS itself fails gracefully.

From my observations, before starting DLSS, applications query the architecture (Turing, Ampere etc) of the active card using NVAPI and do not continue firing up DLSS if the returned architecture is less than Turing, So this should indeed be safe on non-RTX cards.

@jp7677
Copy link
Contributor

jp7677 commented Sep 9, 2022

It would be cool to also include APP-ID 805550 / Assetto Corsa Competizione (#1420) in this list. This game is the reason i started DXVK-NVAPI and serves as my standard test bed :). This game is based on UE4 and runs perfectly with DLSS and also uses D3D11 extensions (SetDepthBoundsTest/UAVOverlap).

@adamnv
Copy link
Contributor Author

adamnv commented Sep 9, 2022

It would be cool to also include APP-ID 805550 / Assetto Corsa Competizione (#1420) in this list. This game is the reason i started DXVK-NVAPI and serves as my standard test bed :). This game is based on UE4 and runs perfectly with DLSS and also uses D3D11 extensions (SetDepthBoundsTest/UAVOverlap).

added - thanks!

@ivyl
Copy link
Collaborator

ivyl commented Sep 14, 2022

It would be good to have also the full versions of the game tested, but that's something we can add based on the demos.

I hope to (re)test just about everything in the fullness of time (and steam sales laughing) but I'm happy to add the non-demos on the basis of the success with the demos; are you saying I should go ahead with that in this PR?

Yes. Adding all of them sounds good. Then we can knock off any games that prove to be problematic as we sift through them.

Just to understand the extent of testing you have done, have you tried any of the games with older, non-RTX GPUs?

On the one hand - no, I haven't! On the other hand, I'm familiar enough with the failure modes of the only dxvknvapi-enabled tech which overlaps with RTX cards - DLSS - to be adequately confident that DLSS itself fails gracefully.

I don't have a reason to distrust you here but I also don't fully trust any technology :-P

I think that landing it in bleeding-edge (the staging tree) would be fine but we will do some sanity checks on this before it makes its way to experimental or stable.

It should not negatively affect games running with other vendors' GPUs but it would be nice to make sure that's the case.

I agree; FWIW I believe dxvknvapi is designed by default to do nothing outside of the NVIDIA driver stack unless a user is explicitly spoofing a customVendorId, but I'm checking that assumption again with @jp7677 grin edit: confirmed by Jens - dxvknvapi does nothing outside of the NVIDIA driver stack by default, regardless of PROTON_ENABLE_NVAPI.

The comment I made above applies here as well.

Would you like me to retarget the PR against another branch or is this one okay?

This one is okay. I apply the PRs manually anyway to the staging tree.

@adamnv
Copy link
Contributor Author

adamnv commented Sep 14, 2022

Cool - I'll get on it today.

@adamnv
Copy link
Contributor Author

adamnv commented Sep 14, 2022

Yes. Adding all of them sounds good.

Done. Cheers.

@adamnv
Copy link
Contributor Author

adamnv commented Oct 7, 2022

I've pushed a commit adding 7 more tested titles.

Going forward, should I continue to extend this PR/branch on an ongoing basis, or issue a new PR for every update? Or something else? :)

I can see this trickling-in of titles extending indefinitely due to the nature of the whitelisting combined with wishing to stay responsive to new releases/updates.
Cheers.

@ivyl
Copy link
Collaborator

ivyl commented Oct 7, 2022

The previous patch is in experimental, so you can just keep on creating new PRs targeting that. No need to that that for this commit though. I'll pick it up and close this PR.

I'm tempted to try enabling nvapi for all games with the next Proton rebase. Have you found any games that would be broken with that? Not even talking about DLSS, just any game that required nvapihack in the past.

@adamnv
Copy link
Contributor Author

adamnv commented Oct 7, 2022

I added one more, sorry. Happy to move it to a new PR if you've already pulled the trigger on the previous batch. :)

I'm tempted to try enabling nvapi for all games with the next Proton rebase. Have you found any games that would be broken with that? Not even talking about DLSS, just any game that required nvapihack in the past.

It's certainly tempting, though I have found some titles (three or four) that break with that. Maybe they're fixable or already fixed at the dxvknvapi level (not sure how closely Proton tracks dxvknvapi master?) or they're just unveiling poor app assumptions but that's another investigation I think.

@ivyl
Copy link
Collaborator

ivyl commented Oct 7, 2022

It's certainly tempting, though I have found some titles (three or four) that break with that. Maybe they're fixable or already fixed at the dxvknvapi level

Do you happen to remember the names? Indeed, those would be interesting to look at / fix.

not sure how closely Proton tracks dxvknvapi master?

I bump dxvk-nvapi whenever Jens asks me to do so, but it wouldn't be hard to start tracking master in experimental. We already do that for dxvk and vkd3d-proton.

@adamnv
Copy link
Contributor Author

adamnv commented Oct 12, 2022

The previous patch is in experimental, so you can just keep on creating new PRs targeting that. No need to that that for this commit though. I'll pick it up and close this PR.

Since I had yet another batch of updates queued up, I've just rolled any outstanding updates from this PR into that one ( #6227 ), so this one can be closed.

@adamnv adamnv closed this Oct 12, 2022
ivyl pushed a commit that referenced this pull request Oct 19, 2022
ivyl pushed a commit that referenced this pull request Oct 28, 2022
ivyl pushed a commit that referenced this pull request Dec 22, 2022
Link: #6120
Link: #6227

(squashed a bunch of commits)
ivyl pushed a commit that referenced this pull request Apr 17, 2023
Link: #6120
Link: #6227

(squashed a bunch of commits)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants